Skip to content

Conversation

@FranciscoTGouveia
Copy link
Contributor

Cherry-picked from #4471, in response to #4471 (comment).

@rami3l rami3l requested review from djc and rami3l October 20, 2025 13:09
Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@djc To clarify, this PR introduces two messages, pending installation for a toolchain waiting to be installed after the download phase, and a spinner to replace the original installing component... line.

Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have a bunch of smaller changes in a single commit. It would be nice to split them apart so that each is individually motivated. (IIRC it was two commits in #4471 in the first place.)

@FranciscoTGouveia
Copy link
Contributor Author

FranciscoTGouveia commented Oct 21, 2025

This seems to have a bunch of smaller changes in a single commit. It would be nice to split them apart so that each is individually motivated. (IIRC it was two commits in #4471 in the first place.)

The two new progress bar states introduced (installing() and installed()) are closely related, so I don’t believe they can be separated into distinct commits.

Additionally, the original PR (4471) contained two commits because it introduced a special case for components that were already downloaded -- which I do not think is necessary.
When installing, the size of the component’s download is already available, and for components that are already installed, that value is simply 0 B.

A screenshot can be seen below to illustrate this:

image

However, I made some changes for the different possible states of the progress bars to be column-aligned and I think those would benefit being in a separate commit.

@djc djc enabled auto-merge October 21, 2025 10:00
@djc
Copy link
Contributor

djc commented Oct 21, 2025

When installing, the size of the component’s download is already available, and for components that are already installed, that value is simply 0 B.

I think it might be better to retain the component's size or drop the size, since 0 B seems kinda wrong.

@djc djc added this pull request to the merge queue Oct 21, 2025
Merged via the queue into rust-lang:main with commit c5839b9 Oct 21, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants